Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

profiles: add auto-generated private-etc lines to all profiles #2093

Closed
wants to merge 1 commit into from
Closed

Conversation

SkewedZeppelin
Copy link
Collaborator

@SkewedZeppelin SkewedZeppelin commented Aug 23, 2018

DO NOT MERGE!

This adds automatically generated private-etc lines to ~380 profiles.
This basically closes #1734

Program source here: https://gist.github.com/SkewedZeppelin/12d5bd24fca41f3c2a4b54d919c8ab44

This shoudn't be merged until private-etc supports globbing.
And we should also wait until after 0.9.56 is released.

Any feedback is appreciated.

@Fred-Barclay
Copy link
Collaborator

Very nice!
Can you break down your program a bit for those of us who don't speak Java? 😆

Does there need to be some fine-tuning still? I'm seeing a lot of things being added (like X11) that I don't recall being needed very much in private-etc?
Also maybe a bit too much is being added? I can't imagine keepassxc actually needed /etc/os-release, for instance.

Cheers!
Fred

@SkewedZeppelin
Copy link
Collaborator Author

SkewedZeppelin commented Aug 23, 2018

Yes, I don't like how much is added either, but 40 items in /etc is way better then 200+. Also considering many profiles lack a private-etc, this is an improvement. Ideas on refining the "detection" are much appreciated.

Feature "detection" is handled by
https://gist.github.com/SkewedZeppelin/12d5bd24fca41f3c2a4b54d919c8ab44#file-firejail-privateetcgenerator-java-L58-L101

and that info gets passed here
https://gist.github.com/SkewedZeppelin/12d5bd24fca41f3c2a4b54d919c8ab44#file-firejail-privateetcgenerator-java-L156-L212

and there is also some special handling here
https://gist.github.com/SkewedZeppelin/12d5bd24fca41f3c2a4b54d919c8ab44#file-firejail-privateetcgenerator-java-L107-L121

@Vincent43
Copy link
Collaborator

I see some obvious issues like dnsmasq not containing dnsmasq.conf and dnsmasq.conf.d. I don't know if this automatic approach is realistic without manually checking app functionality across distros.

@SkewedZeppelin
Copy link
Collaborator Author

@Vincent43 after I finish tweaking the program I plan to go back through and add any missing specific ones.

I have a list of other profiles too:
ardour5, ark, file, seamonkey, etc.

@SkewedZeppelin
Copy link
Collaborator Author

SkewedZeppelin commented Aug 23, 2018

Latest version has private-etc lines commented for all profiles that didn't already have an enabled private-etc line.

All profiles with a previously existing enabled private-etc line have been sanity checked.

I will start going through profiles, uncommenting and testing them, and whitelisting them in the profilesTested array.

@SkewedZeppelin
Copy link
Collaborator Author

Okay, I've tested working ~50 profiles so far.
It should be stable enough for you all to test. Feel free to comment working profiles or just push.

There is still the issue of the large number of added files/paths, but looking at it a bit:

  • it should be safe, most aren't sensitive
  • for profiles that already have a private-etc, it might actually fix minor quirks
  • most of these profiles never had private-etc protections in the first place
  • once globbing is supported, it look like less 😃

Once more profiles have their private-etc enabled then:

  1. we have to wait until after 0.9.56 release
  2. someone still has to add globbing support to private-etc
  3. we can rebase and merge

@smitsohu
Copy link
Collaborator

I think the kde4rc, kde5rc are easier to add manually. It is now in large number of profiles, but only some KDE apps (maybe less than 20) need them.

@SkewedZeppelin
Copy link
Collaborator Author

@smitsohu 20 is still a lot. The program does try to determine if it is a kde/qt app and exclude those if it isn't. But if it is determined to be a "gui" app, then it defaults to including gtk/kde/qt files.

See:
https://gist.github.com/SkewedZeppelin/12d5bd24fca41f3c2a4b54d919c8ab44#file-firejail-privateetcgenerator-java-L76-L84
and
https://gist.github.com/SkewedZeppelin/12d5bd24fca41f3c2a4b54d919c8ab44#file-firejail-privateetcgenerator-java-L240-L253

@smitsohu
Copy link
Collaborator

It's cool, I like our program and the systematic approach. I was only speaking for myself when I said this way round is easier than that way, maybe we can clean up false positives later on if there are any

@SkewedZeppelin
Copy link
Collaborator Author

SkewedZeppelin commented Aug 23, 2018

@smitsohu I have refined the gtk/qt/kde detection. Only 13 profiles now include kde*rc, compared to ~200+ before.

Edit: I think I am back to where I started, there are now many KDE apps missing kde*rc.

@Vincent43
Copy link
Collaborator

@SkewedZeppelin can you tell why os-release and lsb-release are needed for each profile? Currently os-release is used only in one profile (steam) and lsb-release in only two. Even if app want to read those it may work without them as well.

@SkewedZeppelin
Copy link
Collaborator Author

@Vincent43 I've updated it to only include those where they were previously.

Any other files? Do we really need protocol, services, rpc? hostname?

@Vincent43
Copy link
Collaborator

Thx. protocols,services and rpc aren't privacy sensitive so they can stay. As for hostname I wonder if we can add it simultaneously with hostname firejail option.

@reinerh
Copy link
Collaborator

reinerh commented Aug 29, 2018

A few thoughts/suggestions...

To figure out additional files/directories a program could be accessing, try running strings $binary and grep for patterns matching "^/etc/". It is likely those paths will at some point be accessed by the program.
Example:

$ strings /usr/bin/mpv | grep ^/etc
/etc/mpv

If you have a Debian system, you can also check /var/lib/dpkg/info/$PACKAGE.conffiles for conffiles (files in /etc) that are shipped by the package.
For packages that are not currently installed on your system, apt-file is a nice tool (apt-file list $PACKAGE | grep /etc/).
Example:

$ cat /var/lib/dpkg/info/mpv.conffiles 
/etc/mpv/encoding-profiles.conf
$ apt-file list mpv | grep /etc/
mpv: /etc/mpv/encoding-profiles.conf

@SkewedZeppelin
Copy link
Collaborator Author

Okay, I have added support to get files from both strings and apt-list.
Their both vary in their output, and strings can output some useless lines sometimes.

I am going to make a VM with Ubuntu with as many supported programs as possible installed and let it run and hopefully it should get most of our bases covered.

@reinerh
Copy link
Collaborator

reinerh commented Aug 29, 2018

Btw as mentioned, for apt-file you don't need to have the package installed. :-)

@SkewedZeppelin
Copy link
Collaborator Author

SkewedZeppelin commented Aug 29, 2018

Yes, but I'll need them installed for strings for the packages that access files not installed by themselves.

Edit: I think I might need to take a different approach. Create a basic program that runs and outputs any files gathered through the various methods and saves them, and run that on multiple systems instead. And then combine them all.

@SkewedZeppelin
Copy link
Collaborator Author

Okay, so my solution: create two programs.
The first gathers results using multiple methods (strings, dpkg, rpm, pacman, apt-file)
and
the second that combines, processes, and outputs the profiles (rewrite of existing program).

I have created the gatherer program and now just need to rewrite the current program to take its output.

An example output of the gatherer

$ tree dnsmasq
dnsmasq
├── pacman.1535584341604.list
└── strings.1535584341604.list
$ cd dnsmasq
$ cat pacman*
dnsmasq.conf
$ cat strings*
ethers
dnsmasq.conf
hosts
resolv.conf

@SkewedZeppelin
Copy link
Collaborator Author

Okay, I have implemented the two program approach.
The repo is here https://github.com/SkewedZeppelin/FirejailEtcGenerator

The latest version is generated using that new version.
There are still some issues, but it seems to be working okay.

rusty-snake pushed a commit that referenced this pull request Jun 11, 2019
This replaces the outdated templates from #1734
with new templates from the program used in #2093
@vutny
Copy link
Contributor

vutny commented Oct 28, 2019

This PR is being superseded by #2745 , isn't it?

@rusty-snake
Copy link
Collaborator

@vutny #2745 updates the templates in the profile template. This PR adds the template to all profiles.

@vutny
Copy link
Contributor

vutny commented Dec 22, 2019

Aha. Looks like this one is really no merge candidate... Some profiles already have very specific private-etc value. Should we close it?

@SkewedZeppelin
Copy link
Collaborator Author

SkewedZeppelin commented Dec 22, 2019

@vutny I've been busy, but I can rebase it sometime soon probably.

edit: done 🚀

- This includes a workaround for private-etc's lack of globbing support
@reinerh reinerh marked this pull request as draft February 22, 2021 00:40
@SkewedZeppelin SkewedZeppelin closed this by deleting the head repository Jan 11, 2024
@kmk3 kmk3 changed the title Add automatically generated private-etc lines to all profiles profiles: add auto-generated private-etc lines to all profiles Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

private-etc templates
7 participants